awxkit: Support --conf.token/CONTROLLER_OAUTH_TOKEN#16384
awxkit: Support --conf.token/CONTROLLER_OAUTH_TOKEN#16384lalten wants to merge 4 commits intoansible:develfrom
--conf.token/CONTROLLER_OAUTH_TOKEN#16384Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds OAuth2 Bearer-token support to the CLI: config/CLI parsing now supplies a Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Config
participant Auth
participant Session
CLI->>Config: get_config('token')
Config-->>CLI: token
alt token present and non-empty
CLI->>Auth: authenticate()
Auth->>Session: set session.headers['Authorization'] = "Bearer {token}"
Session-->>Auth: header set
Auth-->>CLI: return (early)
else token absent or empty
CLI->>Auth: authenticate()
Auth->>Session: load_session() / login() (basic/session auth)
Session-->>Auth: session established
Auth-->>CLI: return
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
awxkit/test/cli/test_authentication.py (2)
140-155: Assert session path is skipped in all token-auth tests
test_oauth_token_from_env_varandtest_oauth_token_precedence_over_basic_authshould also assertload_sessionis never called, to fully lock in the “Bearer token returns early” behavior.Proposed patch
def test_oauth_token_from_env_var(): @@ assert mock_connection.session.headers['Authorization'] == 'Bearer env_token_value' mock_connection.login.assert_not_called() + mock_root.load_session.assert_not_called() @@ def test_oauth_token_precedence_over_basic_auth(monkeypatch): @@ assert mock_connection.session.headers['Authorization'] == 'Bearer my_token' mock_connection.login.assert_not_called() + mock_root.load_session.assert_not_called()Also applies to: 157-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@awxkit/test/cli/test_authentication.py` around lines 140 - 155, The tests for token-based auth need to assert that session loading is skipped; update test_oauth_token_from_env_var and test_oauth_token_precedence_over_basic_auth to verify load_session is never invoked. Mock or spy on the CLI.load_session (or the instance method used to restore sessions) before calling cli.authenticate(), then add an assertion like cli.load_session.assert_not_called() after authenticate() to lock in the “Bearer token returns early” behavior; reference the CLI.parse_args and CLI.authenticate calls already in the test to locate where to add the mock and assertion.
133-133: Fix Ruff RUF059: unused unpacked variable
mock_rootis unpacked but unused in two tests, which trips lint and adds noise.Proposed patch
- cli, mock_root, mock_connection = setup_token_auth(['awx', '--conf.token', 'cli_token_value']) + cli, _mock_root, mock_connection = setup_token_auth(['awx', '--conf.token', 'cli_token_value']) @@ - cli, mock_root, mock_connection = setup_token_auth(['awx', '--conf.token', 'my_token', '--conf.username', 'user', '--conf.password', 'pass']) + cli, _mock_root, mock_connection = setup_token_auth(['awx', '--conf.token', 'my_token', '--conf.username', 'user', '--conf.password', 'pass'])Also applies to: 159-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@awxkit/test/cli/test_authentication.py` at line 133, The test unpacks a third-party helper return into cli, mock_root, mock_connection but never uses mock_root, triggering Ruff RUF059; update the unpacking in the tests that call setup_token_auth (e.g., where variables cli, mock_root, mock_connection are assigned) to ignore the unused value by renaming it to a throwaway identifier (for example cli, _mock_root, mock_connection or cli, _, mock_connection) or by only assigning the two used values from the tuple, and apply the same change to the other occurrence of the pattern in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@awxkit/test/cli/test_authentication.py`:
- Around line 140-155: The tests for token-based auth need to assert that
session loading is skipped; update test_oauth_token_from_env_var and
test_oauth_token_precedence_over_basic_auth to verify load_session is never
invoked. Mock or spy on the CLI.load_session (or the instance method used to
restore sessions) before calling cli.authenticate(), then add an assertion like
cli.load_session.assert_not_called() after authenticate() to lock in the “Bearer
token returns early” behavior; reference the CLI.parse_args and CLI.authenticate
calls already in the test to locate where to add the mock and assertion.
- Line 133: The test unpacks a third-party helper return into cli, mock_root,
mock_connection but never uses mock_root, triggering Ruff RUF059; update the
unpacking in the tests that call setup_token_auth (e.g., where variables cli,
mock_root, mock_connection are assigned) to ignore the unused value by renaming
it to a throwaway identifier (for example cli, _mock_root, mock_connection or
cli, _, mock_connection) or by only assigning the two used values from the
tuple, and apply the same change to the other occurrence of the pattern in the
file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0463233-939f-46bc-a51c-62f96fbccdbd
📒 Files selected for processing (1)
awxkit/test/cli/test_authentication.py
Document the new --conf.token flag, CONTROLLER_OAUTH_TOKEN and TOWER_OAUTH_TOKEN environment variables, and credential file token support in the authentication and usage documentation.
|
ha, I just saw that this was already added very recently in #16281 and then removed in #16293... @stevensonmichel can you shine some light on what is the plan here? As far as I can see there is no other way than tokens to use the awx cli for SSO users, right? |
|
Token-based auth has been removed, so supporting it in the CLI would not make any sense. The idea is that the identity provider should support tokens, and auth will be via that. |
|
@AlanCoding can you point me at some documentation on how to do this? There is https://cli.github.com/manual/gh_auth_token for GitHub SSO but there is currently no way to inject this auth without a --conf.token/CONTROLLER_OAUTH_TOKEN so that awxkit would use it. When I use this PR to inject a |
2 participants
SUMMARY
Add OAuth2 Bearer token authentication to the awx CLI.
The CLI currently only supports session-based login (
POST /api/login/with username+password) and HTTP Basic auth (viaAWXKIT_FORCE_BASIC_AUTH). Neither works for users who authenticate through social auth providers (e.g. GitHub SSO), since they have no local AWX password.AWX supports creating OAuth2 personal access tokens (User → Tokens → Add), and the API accepts them via
Authorization: Bearer <token>. This PR adds native CLI support for this auth method through:--conf.tokenCLI flagCONTROLLER_OAUTH_TOKEN/TOWER_OAUTH_TOKENenvironment variablestokenfield inAWXKIT_CREDENTIAL_FILEWhen a token is provided, it is sent as a Bearer token in the Authorization header, bypassing session and basic auth entirely.
ISSUE TYPE
COMPONENT NAME
STEPS TO REPRODUCE AND EXTRA INFO
n/a
Summary by CodeRabbit
New Features
Tests
Documentation